contracts/upgradeables/soulbounds/Rewards.sol:66

• Rewards is now UUPS/Initializable but the contract lacks an implementation constructor that disables initializers. Without constructor() { _disableInitializers(); }, someone can initialize the implementation contract directly and grant themselves roles, which is a common UUPS footgun. Also consider importing UUPSUpgradeable from @openzeppelin/contracts-upgradeable/... for consistency with the other upgradeable base contracts.
  contracts/upgradeables/soulbounds/Rewards.sol:403
• _countUniqueErc1155TokenIds() allocates itemIds.length * 10 slots for tracking unique (address,tokenId) pairs. Since there is no hard cap of 10 ERC1155 rewards per item, count++ can exceed the allocated length and revert at runtime. Consider a two-pass approach: first count the total number of ERC1155 reward entries across all itemIds to size the arrays, or maintain explicit tracking of seen pairs in storage when rewards are created.
  contracts/upgradeables/soulbounds/Rewards.sol:240
• getAllTreasuryBalances() can return multiple entries for the same ERC1155 contract address (different token IDs), but the return values don’t include the ERC1155 tokenId for each row. That makes the result ambiguous for consumers (duplicate addresses with different balances/reserved) and hard to act on (e.g., withdraw a specific id). Consider returning a parallel uint256[] tokenIds (0 for ERC20/ERC721) or a struct/tuple that includes (tokenAddress, tokenId, type, balances, metadata) per entry.
  contracts/upgradeables/soulbounds/Rewards.sol:271
• getAllTreasuryBalances() can revert with an out-of-bounds write: totalCount is computed as erc20AndErc721Count + _countUniqueErc1155TokenIds(), but the loop always writes addresses[currentIndex] = tokenAddress even for ERC1155 entries. If only ERC1155 tokens are whitelisted and there are no rewards yet (so totalCount == 0), this write reverts. Move the assignment inside the ERC20/ERC721 branches (or guard with if (currentIndex < totalCount)), and ensure ERC1155-only whitelists return empty arrays rather than reverting.
  contracts/upgradeables/soulbounds/Rewards.sol:128
• Introducing tokenTypes changes runtime assumptions for already-deployed proxies: existing whitelistedTokenList entries (from before this mapping existed) will have tokenTypes[token] == 0 (ETHER), which can cause getAllTreasuryBalances() to miscount/allocate arrays and potentially revert. If this contract is being upgraded in-place, add a migration path (e.g., manager-only function to backfill tokenTypes for existing whitelisted tokens) or make getAllTreasuryBalances resilient to unset types.
  contracts/upgradeables/soulbounds/Rewards.sol:666
• whitelistToken now allows whitelisting ERC721/ERC1155, but depositToTreasury, getTreasuryBalance, and related treasury helpers still assume the token is ERC20 (they call SafeERC20 / IERC20.balanceOf). This makes it easy to whitelist an NFT and then hit confusing reverts when using treasury ERC20 flows. Consider enforcing _type == LibItems.RewardType.ERC20 inside depositToTreasury (and other ERC20-only helpers) or splitting the API so NFT treasury operations cannot be mixed with ERC20 deposit calls.
  contracts/upgradeables/soulbounds/Rewards.sol:720
• removeTokenFromWhitelist enforces balance == 0 for ERC20 and ERC721, but for ERC1155 it only checks erc1155TotalReserved and does not prevent removing a token while the contract still holds ERC1155 balances. That can make assets harder to discover/operate on (e.g., getAllTreasuryBalances only walks whitelisted tokens) and blocks the ERC1155-specific withdraw helper which requires whitelistedTokens[_token]. If you can’t enumerate all ERC1155 IDs, consider tracking received ERC1155 IDs in storage (via ERC1155Receiver hooks) and preventing removal while any tracked balance > 0, or document/handle the removal semantics explicitly.